[SPARK-33522][SQL] Improve exception messages while handling UnresolvedTableOrView by imback82 · Pull Request #30475 · apache/spark

imback82 · 2020-11-23T21:41:14Z

What changes were proposed in this pull request?

This PR proposes to improve the exception messages while UnresolvedTableOrView is handled based on this suggestion: #30321 (comment).

Currently, when an identifier is resolved to a temp view when a table/permanent view is expected, the following exception message is displayed (e.g., for SHOW CREATE TABLE):

t is a temp view not table or permanent view.

After this PR, the message will be:

t is a temp view. 'SHOW CREATE TABLE' expects a table or permanent view.

Also, if an identifier is not resolved, the following exception message is currently used:

Table or view not found: t

After this PR, the message will be:

Table or permanent view not found for 'SHOW CREATE TABLE': t

or

Table or view not found for 'ANALYZE TABLE ... FOR COLUMNS ...': t

Why are the changes needed?

To improve the exception message.

Does this PR introduce any user-facing change?

Yes, the exception message will be changed as described above.

How was this patch tested?

Updated existing tests.

imback82 · 2020-11-23T21:57:51Z

sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala

      sql("DROP TABLE testcat.db.notbl")
    }
-    assert(ex.getMessage.contains("Table or view not found: testcat.db.notbl"))
+    assert(ex.getMessage.contains("Table or view not found for 'DROP TABLE': testcat.db.notbl"))


This message could be misleading since DROP TABLE supports table and a temporary view. To fix this, we can add allowPermanentView as the following and update Analyzer accordingly:

case class UnresolvedTableOrView( multipartIdentifier: Seq[String], commandName: String, allowTempView: Boolean = true, allowPermanentView: Boolean = true) extends LeafNode { require(allowTempView || allowPermanentView) override lazy val resolved: Boolean = false override def output: Seq[Attribute] = Nil }

such that the exception message can be updated to
Table or temporary view not found for 'DROP TABLE': t.

One downside is that if t is resolved to a view, the message will become:

t is a permanent view. 'DROP TABLE' expects a table or temporary view.

instead of (current message):

Cannot drop a view with DROP TABLE. Please use DROP VIEW instead

, meaning it will lose the "hint" (which we can add it to Unresolved* if required).

@cloud-fan WDYT? If you are OK with introducing allowPermanentView, I can do it as a separate PR.

Sorry I don't get it. In this case, testcat.db.notbl is not a permanent view, but a non-existing relation.

When I read Table or view not found for 'DROP TABLE': t, it sounded like DROP TABLE also supports permanent views.

So, I was suggesting Table or temporary view not found for 'DROP TABLE': t since DROP TABLE requires either a table or temporary view. But, I guess Table or view not found sounds better if we want to point out the non-existing relation.

since DROP TABLE requires either a table or temporary view

Do we have such a test case for v2 DROP TABLE?

v2 DROP TABLE only handles ResolvedTable. If it's resolved to a view, it will be handled in ResolveSessionCatalog and tested here:

spark/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

Lines 1056 to 1070 in fdd6c73

test("drop view using drop table") {

withTable("tab1") {

spark.range(10).write.saveAsTable("tab1")

withView("view1") {

sql("CREATE VIEW view1 AS SELECT * FROM tab1")

val message = intercept[AnalysisException] {

sql("DROP TABLE view1")

}.getMessage

assert(message.contains("Cannot drop a view with DROP TABLE. Please use DROP VIEW instead"))

}

}

}

test("create view with mismatched schema") {

withTable("tab1") {

SparkQA · 2020-11-23T22:26:55Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36175/

SparkQA · 2020-11-23T22:51:32Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/36175/

SparkQA · 2020-11-23T23:13:38Z

Test build #131574 has finished for PR 30475 at commit fd2eb37.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-11-24T03:52:30Z

Test build #131609 has finished for PR 30475 at commit b97c925.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-11-24T04:11:59Z

Test build #131578 has finished for PR 30475 at commit fd2eb37.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

…or_view

SparkQA · 2020-11-24T18:27:14Z

Test build #131686 has finished for PR 30475 at commit 68ee277.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class TruncateTable(

imback82 · 2020-11-25T06:51:03Z

retest this please

SparkQA · 2020-11-25T07:15:42Z

Test build #131748 has finished for PR 30475 at commit 68ee277.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class TruncateTable(

…or_view

imback82 · 2020-11-25T18:33:28Z

retest this please

SparkQA · 2020-11-25T19:14:24Z

Test build #131802 has finished for PR 30475 at commit 68ee277.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class TruncateTable(

…or_view

SparkQA · 2020-11-25T22:00:15Z

Test build #131808 has finished for PR 30475 at commit 147c654.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-11-26T01:39:37Z

Test build #131813 has finished for PR 30475 at commit 82cda10.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan

LGTM. Please fix the conflicts.

…or_view

SparkQA · 2020-11-27T09:22:07Z

Test build #131872 has finished for PR 30475 at commit 46a4c4e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CoalesceShufflePartitions(session: SparkSession) extends CustomShuffleReaderRule
trait CustomShuffleReaderRule extends Rule[SparkPlan]

cloud-fan · 2020-11-27T09:44:43Z

retest this please

cloud-fan · 2020-11-27T10:16:54Z

GA passed, merging to master, thanks!

SparkQA · 2020-11-27T13:18:54Z

Test build #131878 has finished for PR 30475 at commit 46a4c4e.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class CoalesceShufflePartitions(session: SparkSession) extends CustomShuffleReaderRule
trait CustomShuffleReaderRule extends Rule[SparkPlan]

initial commit

fd2eb37

github-actions bot added the SQL label Nov 23, 2020

imback82 commented Nov 23, 2020

View reviewed changes

imback82 changed the title ~~[SPARK-33522][SQL] Improve exception messages while handling UnresolvedTable~~ [SPARK-33522][SQL] Improve exception messages while handling UnresolvedTableOrView Nov 24, 2020

Fix tests

b97c925

Merge remote-tracking branch 'upstream/master' into unresolved_table_…

68ee277

…or_view

Merge remote-tracking branch 'upstream/master' into unresolved_table_…

32afb74

…or_view

imback82 added 3 commits November 25, 2020 11:19

Merge remote-tracking branch 'upstream/master' into unresolved_table_…

9ef6ca7

…or_view

Fix compilation

147c654

fix tests

82cda10

cloud-fan approved these changes Nov 27, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into unresolved_table_…

46a4c4e

…or_view

cloud-fan mentioned this pull request Nov 27, 2020

[SPARK-33452][SQL] Support v2 SHOW PARTITIONS #30398

Closed

cloud-fan closed this in 2c41d9d Nov 27, 2020

imback82 mentioned this pull request Dec 8, 2020

[SPARK-33685][SQL] Migrate DROP VIEW command to use UnresolvedView to resolve the identifier #30636

Closed

	test("drop view using drop table") {
	withTable("tab1") {
	spark.range(10).write.saveAsTable("tab1")
	withView("view1") {
	sql("CREATE VIEW view1 AS SELECT * FROM tab1")
	val message = intercept[AnalysisException] {
	sql("DROP TABLE view1")
	}.getMessage
	assert(message.contains("Cannot drop a view with DROP TABLE. Please use DROP VIEW instead"))
	}
	}
	}

	test("create view with mismatched schema") {
	withTable("tab1") {

Conversation

imback82 commented Nov 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

imback82 Nov 23, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

imback82 Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

imback82 Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 23, 2020

Uh oh!

SparkQA commented Nov 23, 2020

Uh oh!

SparkQA commented Nov 23, 2020

Uh oh!

SparkQA commented Nov 24, 2020

Uh oh!

SparkQA commented Nov 24, 2020

Uh oh!

SparkQA commented Nov 24, 2020

Uh oh!

imback82 commented Nov 25, 2020

Uh oh!

SparkQA commented Nov 25, 2020

Uh oh!

imback82 commented Nov 25, 2020

Uh oh!

SparkQA commented Nov 25, 2020

Uh oh!

SparkQA commented Nov 25, 2020

Uh oh!

SparkQA commented Nov 26, 2020

Uh oh!

cloud-fan left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 27, 2020

Uh oh!

cloud-fan commented Nov 27, 2020

Uh oh!

cloud-fan commented Nov 27, 2020

Uh oh!

SparkQA commented Nov 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

imback82 commented Nov 23, 2020 •

edited

Loading